Skip to content

[HLSL][SPIRV] Allow large z value in numthreads #144934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 23, 2025
Merged

Conversation

s-perron
Copy link
Contributor

The current validation checks for numthreads assume that the target is
DXIL so the version checks inadvertently issue error when targeting
SPIR-V.

The current validation checks for numthreads assume that the target is
DXIL so the version checks inadvertently issue error when targeting
SPIR-V.
@s-perron s-perron requested a review from Keenuts June 19, 2025 17:57
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Jun 19, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 19, 2025

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Steven Perron (s-perron)

Changes

The current validation checks for numthreads assume that the target is
DXIL so the version checks inadvertently issue error when targeting
SPIR-V.


Full diff: https://github.com/llvm/llvm-project/pull/144934.diff

2 Files Affected:

  • (modified) clang/lib/Sema/SemaHLSL.cpp (+5-2)
  • (modified) clang/test/SemaHLSL/num_threads.hlsl (+12-4)
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index b55f4fd786b58..9f39c077cea7a 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1033,12 +1033,15 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
 void SemaHLSL::handleNumThreadsAttr(Decl *D, const ParsedAttr &AL) {
   llvm::VersionTuple SMVersion =
       getASTContext().getTargetInfo().getTriple().getOSVersion();
+  bool IsDXIL = getASTContext().getTargetInfo().getTriple().getArch() ==
+                llvm::Triple::dxil;
+
   uint32_t ZMax = 1024;
   uint32_t ThreadMax = 1024;
-  if (SMVersion.getMajor() <= 4) {
+  if (IsDXIL && SMVersion.getMajor() <= 4) {
     ZMax = 1;
     ThreadMax = 768;
-  } else if (SMVersion.getMajor() == 5) {
+  } else if (IsDXIL && SMVersion.getMajor() == 5) {
     ZMax = 64;
     ThreadMax = 1024;
   }
diff --git a/clang/test/SemaHLSL/num_threads.hlsl b/clang/test/SemaHLSL/num_threads.hlsl
index b5f9ad6c33cd6..96200312bbf69 100644
--- a/clang/test/SemaHLSL/num_threads.hlsl
+++ b/clang/test/SemaHLSL/num_threads.hlsl
@@ -10,6 +10,8 @@
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel5.0-compute -x hlsl -ast-dump -o - %s -DFAIL -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel4.0-compute -x hlsl -ast-dump -o - %s -DFAIL -verify
 
+// RUN: %clang_cc1 -triple spirv-pc-vulkan1.3-compute -x hlsl -ast-dump -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-SPIRV
+
 #if __SHADER_TARGET_STAGE == __SHADER_STAGE_COMPUTE || __SHADER_TARGET_STAGE == __SHADER_STAGE_MESH || __SHADER_TARGET_STAGE == __SHADER_STAGE_AMPLIFICATION || __SHADER_TARGET_STAGE == __SHADER_STAGE_LIBRARY
 #ifdef FAIL
 
@@ -88,24 +90,30 @@ int entry() {
 
 // Because these two attributes match, they should both appear in the AST
 [numthreads(2,2,1)]
-// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:90:2, col:18> 2 2 1
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:18> 2 2 1
 int secondFn();
 
 [numthreads(2,2,1)]
-// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:94:2, col:18> 2 2 1
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:18> 2 2 1
 int secondFn() {
   return 1;
 }
 
 [numthreads(4,2,1)]
-// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:100:2, col:18> 4 2 1
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:18> 4 2 1
 int onlyOnForwardDecl();
 
-// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:100:2, col:18> Inherited 4 2 1
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:18> Inherited 4 2 1
 int onlyOnForwardDecl() {
   return 1;
 }
 
+#ifdef __spirv__ 
+[numthreads(4,2,128)]
+// CHECK-SPIRV: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:20> 4 2 128
+int largeZ();
+#endif 
+
 #else // Vertex and Pixel only beyond here
 // expected-error-re@+1 {{attribute 'numthreads' is unsupported in '{{[A-Za-z]+}}' shaders, requires one of the following: compute, amplification, mesh}}
 [numthreads(1,1,1)]

@s-perron s-perron requested a review from Keenuts June 20, 2025 20:32
@s-perron s-perron merged commit cccb82e into llvm:main Jun 23, 2025
12 checks passed
miguelcsx pushed a commit to miguelcsx/llvm-project that referenced this pull request Jun 23, 2025
The current validation checks for numthreads assume that the target is
DXIL so the version checks inadvertently issue error when targeting
SPIR-V.
Jaddyen pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 23, 2025
The current validation checks for numthreads assume that the target is
DXIL so the version checks inadvertently issue error when targeting
SPIR-V.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants